Skip to content

Conversation

@shichengripple001
Copy link
Collaborator

@shichengripple001 shichengripple001 commented Oct 16, 2025

High Level Overview of Change

Revamp release pipeline to run tests, scan for vulnerabilities, request for review

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Did you update CHANGELOG.md?

  • Yes
  • No, this change does not impact library users

Test Plan

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
.github/workflows/faucet_test.yml (1)

1-52: Consider exposing workflow inputs for caller customization.

Adding workflow_call allows reuse from the release pipeline, but the workflow currently defines no inputs. This means external workflows cannot customize the Python versions or other parameters. If the release pipeline needs flexibility (e.g., testing against a subset of Python versions), consider exposing inputs in the workflow_call trigger.

Example enhancement:

 on:
   push:
     branches: [main]
   workflow_dispatch:
   workflow_call:
+    inputs:
+      python-versions:
+        description: "Python versions to test (JSON array)"
+        type: string
+        default: '["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14"]'

Then update line 19 to use the input:

-        python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14"]
+        python-version: ${{ fromJson(inputs.python-versions) }}

This is optional; if the faucet tests always run with the full matrix, no changes are needed.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 651c9ae and 7f256fc.

📒 Files selected for processing (2)
  • .github/workflows/faucet_test.yml (1 hunks)
  • .github/workflows/integration_test.yml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/integration_test.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Integration test (3.11)
  • GitHub Check: Integration test (3.10)
  • GitHub Check: Integration test (3.8)
  • GitHub Check: Integration test (3.12)
  • GitHub Check: Integration test (3.9)
  • GitHub Check: Integration test (3.14)
  • GitHub Check: Integration test (3.13)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
.github/workflows/faucet_test.yml (1)

7-7: ✓ Correct addition of workflow_call trigger.

The workflow_call trigger syntax is valid and allows this workflow to be invoked by other workflows (e.g., the release pipeline). This change aligns with the PR's goal of creating a reusable, multi-stage release workflow.

@@ -0,0 +1,92 @@
# xrpl-py Release Playbook
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Since we have this automated release steps in place here, let us remove those from CONTRIBUTING.md
  2. We should move manual release steps from CONTRIBUTING.md to this file (in case pipeline fails and we need to release the package manually). I think CONTRIBUTING.md should not have any release steps.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case pipeline fails and we need to release the package manually

The manual release workflow publish_to_pypi.yml has been removed. Should we keep it as a backup? Also, we need to update it to ensure that the steps are still working.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manual release workflow publish_to_pypi.yml has been removed. Should we keep it as a backup?

This was also a light weight automation. The publish steps is still happening from the CI. I think if one wants to release it manually they can follow the steps that are currently mentioned in CONTRIBUTING.md

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this workflow fail, publish_to_pypi.yml should also fail. they both use trusted publisher, I think manual release should run from local, we should just assume github action went down.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks both for the clarification. In that case, as Raj suggested, should we update the release-process
section to remove the automated release instructions and keep only the manual release steps? We could also move those steps to a RELEASE.md file or simply reference them as a backup process for situations where automation can’t be used.

uses: actions/setup-python@v5
with:
# Use the lowest supported version of Python for CI/CD
python-version: "3.9"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the version be determined programmatically? It will change over time.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the lowest version supported is 3.8.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to use 3.8 for packaging step

@@ -0,0 +1,92 @@
# xrpl-py Release Playbook
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case pipeline fails and we need to release the package manually

The manual release workflow publish_to_pypi.yml has been removed. Should we keep it as a backup? Also, we need to update it to ensure that the steps are still working.

Comment on lines +80 to +81
5. Create any follow-up housekeeping PRs (e.g., bumping `dev` version or
updating docs) if needed.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this file is the source of truth for the release process, can we be specific here about what need to do to complete a release? From https://github.com/XRPLF/xrpl-py/blob/main/CONTRIBUTING.md#release-process, I can see that we will need to do the following additional steps:

  1. Send an email to xrpl-announce.
  2. Post an announcement in the XRPL Discord #python channel with a link to the changes and highlighting key changes.

@Patel-Raj11 Do we need to update reference doc as well?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to above two points.

Do we need to update reference doc as well?

Those gets automatically picked up based on Github tags.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have move the release instructions from CONTRIBUTE.md into RELEASE.md. please take a look. @kuan121 @Patel-Raj11

inputs: >-
./dist/*.tar.gz
./dist/*.whl
- name: Create GitHub Release
Copy link
Collaborator

@Patel-Raj11 Patel-Raj11 Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use this workflow that is readily available to create Github release? I feel the current one is very verbose.
https://github.com/XRPLF/xrpl.js/blob/3e867b09533dfa087643436eaa0c44f688d54dd1/.github/workflows/release.yml#L590

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shichengripple001 Once these comments are resolved , can you please share links to the following here:

  1. Sample Github release tag on your fork.
  2. Sample xrpl-py package that got published.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made change to use the release action

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shichengripple001 The packages on pypi looks good! Can we make sure that Github tags are of the form vx.y.z?
Currently they are of the form x.y.z. This is to be in sync with the tags historically created in xrpl-py

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
.github/workflows/release.yml (2)

576-594: Ensure release tag points to the correct commit.

The gh release create call should use --target $GITHUB_SHA to align the tag with the exact commit that built the artifacts. Currently the tag may default to the default branch HEAD, creating a mismatch.

       - name: Create GitHub Release
         env:
           GITHUB_TOKEN: ${{ github.token }}
         run: |
           set -euo pipefail
           ARGS=(
             "$PACKAGE_VERSION"
             --repo "$GITHUB_REPOSITORY"
             --generate-notes
+            --target "$GITHUB_SHA"
           )
           if [ "${IS_BETA_RELEASE:-false}" = "true" ]; then
             ARGS+=(--prerelease)
           else
             ARGS+=(--latest)
           fi
           gh release create "${ARGS[@]}" || {
               echo "::error::Failed to create release"
               exit 1
             }

43-49: Pin toml-cli version to prevent unexpected breakage.

Line 48 installs toml-cli without a version constraint. This can cause silent failures if a new toml-cli release introduces breaking changes. Pin to a specific version:

       - name: Install toml-cli
         run: |
           set -euo pipefail
           python3 -m venv /tmp/tomlcli
           /tmp/tomlcli/bin/pip install --upgrade pip
-          /tmp/tomlcli/bin/pip install toml-cli
+          /tmp/tomlcli/bin/pip install toml-cli==0.3.2
           echo "/tmp/tomlcli/bin" >> "${GITHUB_PATH}"

(Replace 0.3.2 with your intended version; check https://pypi.org/project/toml-cli/ for the latest stable release.)

🧹 Nitpick comments (5)
RELEASE.md (2)

16-17: Add missing article and backticks for consistency.

Line 16-17 is missing "the" before "version" and [project] should be backtick-enclosed to match other references:

 The workflow automatically differentiates between beta/pre-release versions
-and standard releases by reading version under the [project] section from `pyproject.toml`:
+and standard releases by reading the version under the `[project]` section from `pyproject.toml`:

21-21: Standardize "prerelease" vs "pre-release" terminology.

The file mixes command-line flags (--prerelease, which must match the actual gh CLI) with narrative descriptions. Ensure consistency: use --prerelease (backtick-enclosed) when referring to the flag, and "pre-release" or "prerelease" consistently in prose. Currently lines 21 and 58 use different variants.

Recommend reviewing the actual gh release create documentation to determine the canonical flag name and then ensure all references match.

Also applies to: 58-58

.github/workflows/release.yml (3)

158-162: Align Poetry installation with other workflows for consistency.

The commented-out section (lines 148–157) and the active installation (lines 158–162) differ from how integration_test.yml and unit_test.yml install Poetry. For maintainability, ensure all workflows use the same installation method and cache strategy. Review .github/workflows/integration_test.yml to confirm the standard approach, then apply it here.


193-194: Pin Trivy action version or check for drift.

The Trivy action is pinned to aquasecurity/[email protected], which is good. Verify this version remains compatible with your CI environment and vulnerability databases; if you update Trivy, test the new SBOM scanning output to ensure the report format remains parseable by downstream steps (e.g., the regex at line 221).


623-623: Fix typo: "succesfully" → "successfully".

-          text="xrpl-py ${PACKAGE_VERSION} has been succesfully released and published to pypi. Release URL: ${RELEASE_URL}"
+          text="xrpl-py ${PACKAGE_VERSION} has been successfully released and published to PyPI. Release URL: ${RELEASE_URL}"

Also capitalize "PyPI" for consistency with the rest of the documentation.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f256fc and e9258ed.

📒 Files selected for processing (3)
  • .github/workflows/release.yml (1 hunks)
  • .github/workflows/unit_test.yml (1 hunks)
  • RELEASE.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
RELEASE.md

[uncategorized] ~4-~4: The official name of this software platform is spelled with a capital “H”.
Context: ...? to PyPIGitHub Actions workflow (see.github/workflows/release.yml`). ## 0. Config...

(GITHUB)


[uncategorized] ~21-~21: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... - The GitHub Release is created with --prerelease. - The latest tag on GitHub rema...

(EN_WORD_COHERENCY)


[uncategorized] ~58-~58: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... creates or updates the GitHub Release (--prerelease for beta versions, --latest for stab...

(EN_WORD_COHERENCY)


[uncategorized] ~91-~91: The official name of this software platform is spelled with a capital “H”.
Context: ...tomation needs adjustments, update both .github/workflows/release.yml and this guide s...

(GITHUB)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Integration test (3.8)
  • GitHub Check: Integration test (3.13)
  • GitHub Check: Integration test (3.12)
  • GitHub Check: Integration test (3.14)
  • GitHub Check: Integration test (3.9)
  • GitHub Check: Integration test (3.10)
  • GitHub Check: Integration test (3.11)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
.github/workflows/unit_test.yml (1)

8-8: LGTM! Adding workflow_call enables external invocation, aligning with the release pipeline orchestration. The change is consistent with parallel updates to faucet_test.yml and integration_test.yml.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9258ed and a9d824f.

📒 Files selected for processing (1)
  • .github/workflows/release.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Integration test (3.13)
  • GitHub Check: Integration test (3.12)
  • GitHub Check: Integration test (3.9)
  • GitHub Check: Integration test (3.14)
  • GitHub Check: Integration test (3.8)
  • GitHub Check: Integration test (3.11)
  • GitHub Check: Integration test (3.10)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
.github/workflows/release.yml (4)

155-155: Clarify Python version discrepancy in packaging vs. scanning stages.

Lines 155 and 178 use different Python versions: 3.8 for packaging (line 155: python-version: "3.8") and 3.9 for vulnerability scanning (line 178: python-version: "3.9"). Prior review comments flagged that pyproject.toml declares requires-python = ">=3.8.1" and earlier feedback suggested aligning the workflow with Python 3.9 as the minimum (since 3.8 reached EOL in October 2024).

Please clarify:

  1. Is the 3.8→3.9 version switch intentional (to build with minimum supported Python but scan with a newer version for scanning tool compatibility)?
  2. Or should both use 3.9 if the project has dropped 3.8 support?
  3. If 3.8 is still supported, update pyproject.toml to document this; if not, update both steps to 3.9 and update requires-python accordingly.

Also applies to: 178-178


237-264: ✅ Vulnerability issue creation logic is well-structured.

Lines 237–264 correctly create a GitHub issue when CRITICAL or HIGH vulnerabilities are detected, with proper environment context, templated issue body, and labeling. The conditional gate on line 238 ensures issues are only created when vulnerabilities exist.


498-537: ✅ PyPI publishing permissions are correctly configured.

The publish-to-pypi job (line 514) uses only id-token: write (line 516), which is correct for PyPI Trusted Publishing. The pypa/gh-action-pypi-publish action with attestations: true (line 537) generates and uploads attestations to PyPI automatically using the OIDC token, requiring no additional attestations: write permission. (The pre-release job correctly includes attestations: write on line 123 for its own use of actions/attest-build-provenance on line 166.)


1-624: Overall workflow design and orchestration look solid.

The multi-stage release pipeline correctly chains validation → testing → packaging → vulnerability scanning → approval gates → publishing → notifications. Permissions are properly scoped, artifact flow is clean, and external integrations (OWASP, Slack, PyPI, sigstore) are well-integrated. The prior review feedback on actions/cache@v4, PACKAGE_NAME in Slack, and Python version alignment has been substantially addressed in earlier commits.

Remaining items to resolve:

  1. Python version discrepancy (3.8 vs 3.9) requires clarification per prior comments
  2. Minor typo on line 604 ("succesfully")

Comment on lines +589 to +610
- name: Notify Slack success (single-line)
if: success()
env:
SLACK_TOKEN: ${{ secrets.SLACK_TOKEN }}
REPO: ${{ github.repository }}
PACKAGE_VERSION: ${{ env.PACKAGE_VERSION }}
TAG: ${{ env.PACKAGE_VERSION }}
run: |
set -euo pipefail
# Build release URL from tag (URL-encoded to handle '@' etc.)
TAG="${TAG:-${PACKAGE_VERSION}}"
enc_tag="$(printf '%s' "$TAG" | jq -sRr @uri)"
RELEASE_URL="https://github.com/$REPO/releases/tag/$TAG"
text="xrpl-py ${PACKAGE_VERSION} has been succesfully released and published to pypi. Release URL: ${RELEASE_URL}"
text="${text//\\n/ }"
curl -sS -X POST https://slack.com/api/chat.postMessage \
-H "Authorization: Bearer $SLACK_TOKEN" \
-H "Content-Type: application/json; charset=utf-8" \
-d "$(jq -n --arg channel "#xrpl-py" --arg text "$text" '{channel:$channel, text:$text}')"
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix typo in Slack success message.

Line 604 contains a spelling error: "succesfully" should be "successfully".

-          text="xrpl-py ${PACKAGE_VERSION} has been succesfully released and published to pypi. Release URL: ${RELEASE_URL}"
+          text="xrpl-py ${PACKAGE_VERSION} has been successfully released and published to pypi. Release URL: ${RELEASE_URL}"
🤖 Prompt for AI Agents
In .github/workflows/release.yml around lines 589 to 610 the Slack success
message contains a typo: the text variable uses "succesfully" instead of
"successfully"; update that string to "successfully" (i.e., change "xrpl-py
${PACKAGE_VERSION} has been succesfully released..." to use the correct
spelling) so the posted Slack message reads correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants